Skip to content

Zero fee htlc preparation: Part 2 - #6074

Merged
rustyrussell merged 18 commits into
ElementsProject:masterfrom
rustyrussell:zero-fee-htlc-prep-2
Apr 7, 2023
Merged

Zero fee htlc preparation: Part 2#6074
rustyrussell merged 18 commits into
ElementsProject:masterfrom
rustyrussell:zero-fee-htlc-prep-2

Conversation

@rustyrussell

@rustyrussell rustyrussell commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

(Based on #6073 , starts at hsmd: add support for lightningd signing onchain txs. )

This simply shifts the onchaind logic from "tell lightningd to broadcast a tx for us" to "tell lightningd to create a tx for us". This would be fairly simple, except for three things:

  1. We generalize the "how do onchaind know whether a tx we see is the one we asked you to create" by having lightningd return the witness, with signature parts flagged (since those can change with fee changes).
  2. The tests had many assumptions about the format of messages from onchaind.
  3. The hsmd interface assumed that onchaind would ask for tx sigs directly, but now lightningd does it.

This removes the rbf logic for now: the next step is to bring it back.

Changelog-None No user visible changes

@rustyrussell rustyrussell added this to the v23.05 milestone Mar 7, 2023
@rustyrussell
rustyrussell requested a review from cdecker as a code owner March 7, 2023 00:52
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch from b734de5 to 6a7ee63 Compare March 23, 2023 10:19
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch 3 times, most recently from b8fee58 to db93f41 Compare March 23, 2023 23:20
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch 2 times, most recently from 4e2a4e4 to 5758155 Compare March 24, 2023 03:29
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch from 5758155 to 68168a9 Compare April 1, 2023 03:55
@cdecker cdecker added hsmd VLS Marks issues and PRs that are relevant to VLS labels Apr 5, 2023
@cdecker

cdecker commented Apr 5, 2023

Copy link
Copy Markdown
Member

ACK 68168a9

We previously used WIRE_HSMD_SIGN_DELAYED_PAYMENT_TO_US,
WIRE_HSMD_SIGN_REMOTE_HTLC_TO_US, WIRE_HSMD_SIGN_PENALTY_TO_US and
WIRE_HSMD_SIGN_LOCAL_HTLC_TX which allow onchaind to sign txs,
but only for its specific channel.

We now want lightningd to sign these, but it's not bound to a specific
channel.  So let's add variants that don't require that.

We are also now explicit about *what input* to sign.  It's always zero
for now, but future combinations may change that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We add code for the case of spending a (timelocked) to-us output of an
HTLC output, so lightningd can do it (rather than onchaind doing all
the work itself).

onchaind still needs to know whether we bothered to create the tx
(fees might have caused it to evaporate, so it should consider it
immediately resolved rather than waiting for it), and what the
witnesses were, and which parts of the witnesses were signatures (as
these parts might change, with RBF or in future, combining other txs).

The inputs (known to onchaind) and the witnesses (told by lightningd)
uniquely identify the spend for the purposes of onchaind.  In
particular, they definitely distinguish HTLC-timeout and HTLC-success
cases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we do both our own internal handling and handing it to
lightningd, we add to `proposed_resolution` to handle the lightningd
case.

Note, in particular, that we fix the blockheight calculation: it's out
by one, in that if we see a tx and our CSV lock is 5, we only need to
wait 4 more blocks, not 5.  This will matter as we start using it, and
convert the tests.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…haind uses lightningd for broadcast.

We can no longer grab the tx in one line as we did with
wait_for_onchaind_broadcast, we need to track the broadcast from
lightningd.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This follows the same pattern as the "spend htlc tx" in fact.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We'll use this later to calculate deadlines for spending txs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We'll reimplement it once lightningd makes all the onchain txs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We'll want this, as lightningd will want to produce htlc txs based on
what it's told from onchaind, so we need a lower-level accessor.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ons.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ons.

This breaks tests/test_closing.py::test_onchain_all_dust's accouting
checks.

That test doesn't really test what it claims to test; sure, onchaind
*says* it's going to ignore the output due to high fees, but the tx
still gets mined.

I cannot figure out what the test is supposed to look like, so I
simply disabled the accounting checks :(

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is when they closed the channel, we can simply make our own tx to
expire the HTLC.  (The other case is where we closed the channel, and
we have a special htlc_timeout tx which we have their signature for).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…aind_broadcast

Using single tuples in Python is ugly, so:
1. Rename wait_for_onchaind_tx to wait_for_onchaind_txs.
2. Make it take tuples explicitly.
3. Make wait_for_onchaind_tx a simpler wrapper/unwrapper.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do this for HTLCs which will timeout to them: we watch them in case we
want to fulfill them as a preimage comes in, but once they reach depth we
can forget about them.

We change the message, which causes some more test churn.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch from 68168a9 to 450ae4d Compare April 5, 2023 23:34
@rustyrussell

Copy link
Copy Markdown
Contributor Author

Trivial rebase on master

Ack 450ae4d

@rustyrussell
rustyrussell merged commit a3b81ba into ElementsProject:master Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hsmd VLS Marks issues and PRs that are relevant to VLS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants